Skip to content

DYN-9032 - add debug for dumping node icons#16293

Merged
johnpierson merged 12 commits into
DynamoDS:masterfrom
johnpierson:dyn9032-addDebugForDumpingNodeIcons
Jun 13, 2025
Merged

DYN-9032 - add debug for dumping node icons#16293
johnpierson merged 12 commits into
DynamoDS:masterfrom
johnpierson:dyn9032-addDebugForDumpingNodeIcons

Conversation

@johnpierson

@johnpierson johnpierson commented Jun 11, 2025

Copy link
Copy Markdown
Member

Purpose

Adds a new debug command to export all node icons from the workspace for troubleshooting.

  • Introduces a "Dump Node Icons" menu item under the Debug menu in the main view.
  • Registers a new DumpNodeIconsCommand and implements DumpNodeIconData to iterate through visible nodes and capture their icons.
  • Adds corresponding resource entries for the menu label in both default and en-US .resx files.

image

Declarations

Check these if you believe they are true

  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB
  • This PR introduces new feature code involve network connecting and is tested with no-network mode.

Release Notes

N/A

Reviewers

@QilongTang

FYIs

@DynamoDS/eidos

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-9032

@johnpierson johnpierson marked this pull request as ready for review June 11, 2025 19:55
@johnpierson johnpierson requested a review from Copilot June 11, 2025 19:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds a new debug command to export all node icons from the workspace for troubleshooting.

  • Introduces a "Dump Node Icons" menu item under the Debug menu in the main view.
  • Registers a new DumpNodeIconsCommand and implements DumpNodeIconData to iterate through visible nodes and capture their icons.
  • Adds corresponding resource entries for the menu label in both default and en-US .resx files.

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/DynamoCoreWpf/Views/Core/DynamoView.xaml Added menu item bound to DumpNodeIconsCommand.
src/DynamoCoreWpf/ViewModels/Core/DynamoViewModelDelegateCommands.cs Registered DumpNodeIconsCommand in initialization and as a public property.
src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs Implemented DumpNodeIconData method to place nodes, inspect their icons, and (intended) record results.
src/DynamoCoreWpf/Properties/Resources.resx Defined localized string for the new menu entry.
src/DynamoCoreWpf/Properties/Resources.en-US.resx Added en-US translation for the new menu entry.
Files not reviewed (1)
  • src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (2)

src/DynamoCoreWpf/ViewModels/Core/DynamoViewModelDelegateCommands.cs:96

  • [nitpick] The method name DumpNodeIconData (singular) is inconsistent with the command name DumpNodeIconsCommand (plural). Consider renaming for consistency, e.g. DumpNodeIconsData.
DumpNodeIconsCommand = new DelegateCommand(DumpNodeIconData, CanDumpNodeHelpData);

src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs:4305

  • [nitpick] There are no accompanying unit tests for the new DumpNodeIconData logic. Adding tests would help ensure correct output formatting and command availability.
internal void DumpNodeIconData(object parameter)

Comment thread src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs
Comment thread src/DynamoCoreWpf/ViewModels/Core/DynamoViewModel.cs
Comment thread src/DynamoCoreWpf/ViewModels/Core/DynamoViewModelDelegateCommands.cs Outdated
@johnpierson johnpierson requested a review from QilongTang June 11, 2025 20:03

@QilongTang QilongTang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I guess the nodes list can be exported from graph node manager?

Comment thread src/DynamoCoreWpf/Properties/Resources.Designer.cs
@johnpierson

Copy link
Copy Markdown
Member Author

LGTM, I guess the nodes list can be exported from graph node manager?

For now, it writes a csv to the logs folder

@johnpierson

Copy link
Copy Markdown
Member Author

revised to include assembly in the export as well

@QilongTang

Copy link
Copy Markdown
Contributor

LGTM, I guess the nodes list can be exported from graph node manager?

For now, it writes a csv to the logs folder

Oh missed that, with that, would you attach the CreateRealTimeInfoWindow() call to let devs know where to find the csv? Thanks, otherwise, LGTM

internal void DumpNodeIconData(object parameter)
{
//set to manual run mode
this.HomeSpace.RunSettings.RunType = RunType.Manual;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For manual run mode? Because I’m placing real nodes to get their view model which reports the icon source. Node model and node search element don’t report icon source. And placing real nodes causes hangups if on run automatic because of default values.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did I understand this correctly - you're placing nodes in the canvas that don't have icons to identify them so icons can be added for them?

@johnpierson johnpierson Jun 12, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my knowledge, one of the easiest ways to find if a node has an icon is to get its view model and get ‘IconSource’, so I’m placing the nodes on the canvas to get that and it also serves as a way for the end user to see the nodes themselves.

This is all for a tool to help identify nodes with missing icons.

@johnpierson

Copy link
Copy Markdown
Member Author

image

@johnpierson johnpierson requested a review from QilongTang June 12, 2025 15:31
@johnpierson johnpierson merged commit 0dd96e3 into DynamoDS:master Jun 13, 2025
26 of 27 checks passed
@johnpierson johnpierson deleted the dyn9032-addDebugForDumpingNodeIcons branch June 13, 2025 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants